Move projection registration out of Khepri setup #11879
Merged
+90
−61
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
the-mikedavis
changed the title
the-mikedavis
force-pushed
the
md/khepri/register-projections-khepri-only
branch
from
5225381 to
90e0fd0
Compare
the-mikedavis
changed the title
the-mikedavis
force-pushed
the
md/khepri/register-projections-khepri-only
branch
from
86f2651 to
348c6d4
Compare
the-mikedavis
changed the title
dumbbell
requested changes
Aug 12, 2024
Collaborator
Do you want to wait for the |
Collaborator
Author
|
Yeah let's wait for that since we also want the |
Collaborator
|
Khepri 0.15.0 is now released and used in RabbitMQ |
the-mikedavis
force-pushed
the
md/khepri/register-projections-khepri-only
branch
2 times, most recently
from
2b247e3 to
56f4489
Compare
dumbbell
requested changes
Sep 9, 2024
Collaborator
dumbbell
left a comment
dumbbell
left a comment
There was a problem hiding this comment.
The patch looks good to me. I just made a minor style-related comment.
Previously this function threw errors. With this minor refactor we return them instead so that `register_projection/0` is easier for callers to work with. (In the child commit we will add another caller.)
This is a cosmetic change. `?RA_CLUSTER_NAME` is equivalent but is used for clustering commands. Commands sent via the `khepri`/`khepri_adv` APIs consistently use the `?STORE_ID` macro instead.
This causes a clearer error when the `enable_feature_flags/2` function
returns something not in the shape `ok | {skip, any()}`.
This covers a specific case where we need to register projections not covered by the enable callback of the `khepri_db` feature flag. The feature flag may be enabled if a node has been part of a cluster which enabled the flag, but the metadata store might be reset. Upon init the feature flag will be enabled but the store will be empty and the projections will not exist, so operations like inserting default data will fail when asserting that a vhost exists for example. This fixes the `cluster_management_SUITE:forget_cluster_node_in_khepri/1` case when running the suite with `RABBITMQ_METADATA_STORE=khepri`, which fails as mentioned above. We could run projection registration always when using Khepri but once projections are registered the command is idempotent so there's no need to, and the commands are somewhat large.
`khepri:fence/0,1,2` queries the leader's Raft index and blocks the caller for the given (or default) timeout until the local member has caught up in log replication to that index. We want to do this during Khepri init to ensure that the local Khepri store is reasonably up to date before continuing in the boot process and starting listeners. This is conceptually similar to the call to `mnesia:wait_for_tables/2` during `rabbit_mnesia:init/0` and should have the same effect.
the-mikedavis
force-pushed
the
md/khepri/register-projections-khepri-only
branch
from
6870cf1 to
ce72903
Compare
Collaborator
Author
|
I'll set up the backports once we have the prerequisite PRs backported. In particular this PR needs #11672 |
This was referenced Sep 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
rabbit_khepri:setup/0is always called during boot for the sake of clustering. Previously we registered projections duringrabbit_khepri:setup/0meaning that even if you run Rabbit 3.13.x without enabling thekhepri_dbfeature flag you will attempt to register projections. It's a benign bug - it doesn't cause any bad behavior other than a potential timeout on boot if a server is in a minority - but it modifies the Khepri store so we need to consider the projections when we reorganize the tree (#11225).Instead of registering projections during
rabbit_khepri:setup/0we should register them in two spots:rabbit_db:init/0), a boot step run afterrabbit_khepri:setup/0which has a Khepri-specific branch, and only if we detect that the database is blank. (We could always register on init when Khepri is enabled but there's no need to unless the database is blank.)khepri_dbfeature flag.In the future when we want to unregister projections, we can add that step before
rabbit_khepri:register_projections/0to the enable callback for thekhepri_dbfeature flag (using rabbitmq/khepri#282).